Refactor frontend file validation and hints - #3235
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBackend upload validation now uses fluent Laravel rules and shared image constraints. BigBlueButton file MIME settings are normalized to arrays, while frontend selectors, error handling, upload hints, admin views, and related tests are updated. ChangesFile Upload Hints and Validation Refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3235 +/- ##
=============================================
- Coverage 97.10% 97.07% -0.04%
- Complexity 1949 1952 +3
=============================================
Files 483 485 +2
Lines 16660 16727 +67
Branches 2408 2421 +13
=============================================
+ Hits 16178 16238 +60
- Misses 482 489 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PILOS
|
||||||||||||||||||||||||||||||||||
| Project |
PILOS
|
| Branch Review |
file-hint-and-validation
|
| Run status |
|
| Run duration | 07m 46s |
| Commit |
|
| Committer | Samuel Weirich |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
1
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
634
|
| View all changes introduced in this branch ↗︎ | |
Tests for review

e2e/RoomsJoinWithLobby.cy.js • 1 failed test • System tests
| Test | Artifacts | |
|---|---|---|
| Room Join with lobby settings > Lobby enabled for guests only |
Test Replay
Screenshots
|
|
05b517d to
d07bcc0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/components/SettingsFileSelector.vue (1)
108-110: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winGuard
fileErrorbefore readinglength.frontendFileErrorstill callsprops.fileError.length, but the prop defaults tonull, so the component throws on the first render when no backend file errors are present. Default it to[](and keep the prop type array-based) or null-check first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/components/SettingsFileSelector.vue` around lines 108 - 110, The `fileError` prop in `SettingsFileSelector.vue` can be null by default, but `frontendFileError` still reads `props.fileError.length`, causing a first-render crash. Update the `fileError` prop definition to use an array-based default (and matching type) or add a null guard inside `frontendFileError` before accessing `length`, so the computed logic in `frontendFileError` and related file-error handling works safely when no backend errors are present.
🧹 Nitpick comments (2)
resources/js/composables/useFormErrors.js (1)
48-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the rest of the
fieldErrorJSDoc to match the new array contract.The new
@return {string[]}is right, but the same block still says this helper returns HTML forv-html. That no longer matches the implementation or the newFormErrorusage, so the docs will send the next caller down the wrong path.📝 Suggested doc update
- * Returns the error message for the passed field. - * - * Since a html list gets returned for the passed field the string cannot be used directly in the - * html code but the v-html tag must be used. + * Returns the validation messages for the passed field. * * `@example` - * <b-form-group :state="fieldState('...')" ...> - * <template `#invalid-feedback`><div v-html="fieldError('...')"></div></template> - * </b-form-group> - * - * <!-- or --> - * <b-form-invalid-feedback :state="fieldState('...')" v-html="fieldError('...')"></b-form-invalid-feedback> + * <FormError :errors="fieldError('...')" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/composables/useFormErrors.js` around lines 48 - 54, The JSDoc for fieldError in useFormErrors.js still describes HTML/v-html output even though the method now returns a string array, so update the remaining comments in that block to match the array contract. Keep the `@return` {string[]} description aligned with the implementation and FormError usage, and remove or rewrite any text that implies the helper returns HTML or is meant for v-html.app/Http/Requests/StoreRoomFileRequest.php (1)
13-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
: arraytoStoreRoomFileRequest::rules()The method still lacks an explicit return type, which conflicts with the PHP typing guideline and makes the validation contract less clear.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Requests/StoreRoomFileRequest.php` around lines 13 - 14, StoreRoomFileRequest::rules() is missing an explicit return type, so update the rules method signature to declare it returns an array. Keep the existing validation rule contents unchanged and ensure the method declaration in StoreRoomFileRequest matches the PHP typing guideline.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Http/Requests/UserRequest.php`:
- Line 31: The image validation in UserRequest currently only accepts JPG, but
the profile UI uploads the cropped avatar as image.png, so the request is
rejected. Update the image rule in UserRequest to match the filename/format sent
from UserTabProfile, or adjust the client upload naming so the validator and
upload flow stay consistent. Keep the existing size, dimension, and Antivirus
checks intact while fixing the extension/type mismatch.
In `@config/bigbluebutton.php`:
- Around line 7-16: The allowed file MIME list in the bigbluebutton config is
preserving whitespace from the BBB_ALLOWED_FILE_MIMES env value, so tokens like
" docx" can leak into both validation and the upload hint text. Update the
allowedFileMimes parsing in the config to trim each comma-separated token before
filtering/uniquing, and keep the fallback behavior unchanged so
allowed_file_mimes always contains clean extension strings.
---
Outside diff comments:
In `@resources/js/components/SettingsFileSelector.vue`:
- Around line 108-110: The `fileError` prop in `SettingsFileSelector.vue` can be
null by default, but `frontendFileError` still reads `props.fileError.length`,
causing a first-render crash. Update the `fileError` prop definition to use an
array-based default (and matching type) or add a null guard inside
`frontendFileError` before accessing `length`, so the computed logic in
`frontendFileError` and related file-error handling works safely when no backend
errors are present.
---
Nitpick comments:
In `@app/Http/Requests/StoreRoomFileRequest.php`:
- Around line 13-14: StoreRoomFileRequest::rules() is missing an explicit return
type, so update the rules method signature to declare it returns an array. Keep
the existing validation rule contents unchanged and ensure the method
declaration in StoreRoomFileRequest matches the PHP typing guideline.
In `@resources/js/composables/useFormErrors.js`:
- Around line 48-54: The JSDoc for fieldError in useFormErrors.js still
describes HTML/v-html output even though the method now returns a string array,
so update the remaining comments in that block to match the array contract. Keep
the `@return` {string[]} description aligned with the implementation and FormError
usage, and remove or rewrite any text that implies the helper returns HTML or is
meant for v-html.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: acb204f1-1d4f-4132-b0e4-0a9cff0e6e12
📒 Files selected for processing (26)
CHANGELOG.mdapp/Http/Requests/StoreRoomFileRequest.phpapp/Http/Requests/UpdateRoomStreamingConfigRequest.phpapp/Http/Requests/UpdateRoomTypeStreamingSettingsRequest.phpapp/Http/Requests/UpdateSettingsRequest.phpapp/Http/Requests/UpdateStreamingSettingsRequest.phpapp/Http/Requests/UserRequest.phpconfig/bigbluebutton.phplang/en/app.phplang/en/rooms.phpresources/js/components/AdminStreamingRoomTypeEditButton.vueresources/js/components/FormError.vueresources/js/components/RoomTabFilesUploadButton.vueresources/js/components/RoomTabStreamingConfigButton.vueresources/js/components/SettingsFileSelector.vueresources/js/components/SettingsImageSelector.vueresources/js/components/UserProfileImageSelector.vueresources/js/composables/useFileHelpers.jsresources/js/composables/useFormErrors.jsresources/js/views/AdminRoomTypesView.vueresources/js/views/AdminSettings.vueresources/js/views/AdminStreamingSettings.vuetests/Backend/Feature/api/v1/ApplicationTest.phptests/Backend/Feature/api/v1/SettingsTest.phptests/Frontend/e2e/RoomsViewFilesFileActions.cy.jstests/Frontend/fixtures/config.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/components/LoginTabLocal.vue`:
- Around line 104-111: The watcher on props.errors in LoginTabLocal.vue
currently only updates formErrors after a change, so the initial error state is
missed on mount. Update the watch call that uses toRaw(newErrors) and
formErrors.set(...) to run immediately by adding immediate: true, so existing
props.errors are applied as soon as the component initializes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9f175c47-42f0-4dbc-9c39-170e74d9fbe9
📒 Files selected for processing (17)
resources/js/components/AdminStreamingRoomTypeEditButton.vueresources/js/components/FormError.vueresources/js/components/LoginTabLdap.vueresources/js/components/LoginTabLocal.vueresources/js/components/RoomTabSettingsAccessCodeInput.vueresources/js/components/RoomTabSettingsRadioGroup.vueresources/js/components/RoomTabSettingsRoomTypeSelect.vueresources/js/components/RoomTabSettingsSelectButton.vueresources/js/components/RoomTabSettingsTextArea.vueresources/js/components/RoomTabSettingsToggleSwitch.vueresources/js/components/RoomTabStreamingConfigButton.vueresources/js/components/SettingsFileSelector.vueresources/js/components/SettingsImageSelector.vueresources/js/composables/useFormErrors.jsresources/js/views/AdminSettings.vueresources/js/views/AdminStreamingSettings.vueresources/js/views/Login.vue
💤 Files with no reviewable changes (1)
- resources/js/components/FormError.vue
✅ Files skipped from review due to trivial changes (1)
- resources/js/components/RoomTabSettingsAccessCodeInput.vue
🚧 Files skipped from review as they are similar to previous changes (6)
- resources/js/components/RoomTabStreamingConfigButton.vue
- resources/js/components/AdminStreamingRoomTypeEditButton.vue
- resources/js/views/AdminStreamingSettings.vue
- resources/js/components/SettingsFileSelector.vue
- resources/js/composables/useFormErrors.js
- resources/js/components/SettingsImageSelector.vue
| { | ||
| return [ | ||
| 'file' => ['bail', 'required', 'file', 'max:'.(config('bigbluebutton.max_filesize') * 1000), 'mimes:'.config('bigbluebutton.allowed_file_mimes'), new Antivirus], // https://github.com/bigbluebutton/bigbluebutton/blob/v2.2.x-release/bigbluebutton-html5/private/config/settings.yml | ||
| 'file' => ['bail', 'required', File::types(config('bigbluebutton.allowed_file_mimes'))->extensions(config('bigbluebutton.allowed_file_mimes'))->max(config('bigbluebutton.max_filesize').'mb'), new Antivirus], // https://github.com/bigbluebutton/bigbluebutton/blob/v2.2.x-release/bigbluebutton-html5/private/config/settings.yml |
There was a problem hiding this comment.
Link in the comment not working anymore
| if (fileTooBig.value) { | ||
| errors.push(t("app.validation.too_large")); | ||
| } | ||
| if (fileInvalidExtension.value) { | ||
| errors.push(t("app.validation.invalid_type")); | ||
| } |
There was a problem hiding this comment.
| if (fileTooBig.value) { | |
| errors.push(t("app.validation.too_large")); | |
| } | |
| if (fileInvalidExtension.value) { | |
| errors.push(t("app.validation.invalid_type")); | |
| } | |
| if (fileTooBig.value) { | |
| errors.push(t("app.file.too_large")); | |
| } | |
| if (fileInvalidExtension.value) { | |
| errors.push(t("app.file.invalid_type")); | |
| } |
Wrong translation key
| if (fileTooBig.value) { | ||
| errors.push(t("app.validation.too_large")); | ||
| } | ||
| if (fileInvalidExtension.value) { | ||
| errors.push(t("app.validation.invalid_type")); | ||
| } |
There was a problem hiding this comment.
| if (fileTooBig.value) { | |
| errors.push(t("app.validation.too_large")); | |
| } | |
| if (fileInvalidExtension.value) { | |
| errors.push(t("app.validation.invalid_type")); | |
| } | |
| if (fileTooBig.value) { | |
| errors.push(t("app.file.too_large")); | |
| } | |
| if (fileInvalidExtension.value) { | |
| errors.push(t("app.file.invalid_type")); | |
| } |
Wrong translation key
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Rules/Image.php`:
- Around line 13-14: The Image::default() validation rule currently permits SVG
uploads without the required sanitization and isolation safeguards. Remove svg
from the types and extensions lists in Image::default(), and remove the
corresponding SVG choices from matching upload UI options; do not add SVG
support unless it is separately sanitized and served from an isolated origin
with safe response headers.
In `@tests/Backend/Feature/api/v1/Room/RoomStreamingTest.php`:
- Line 59: Update the MIME-mismatch fixtures at
tests/Backend/Feature/api/v1/Room/RoomStreamingTest.php:59 and
tests/Backend/Feature/api/v1/StreamingTest.php:35 to use a supported image
extension such as .jpg while overriding the generated file’s MIME type to
image/tiff; preserve the existing image dimensions and test intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b123e25f-9a26-41c2-a875-6437a8678b94
📒 Files selected for processing (14)
app/Http/Requests/UpdateRoomStreamingConfigRequest.phpapp/Http/Requests/UpdateRoomTypeStreamingSettingsRequest.phpapp/Http/Requests/UpdateSettingsRequest.phpapp/Http/Requests/UpdateStreamingSettingsRequest.phpapp/Http/Requests/UserRequest.phpapp/Rules/Image.phpresources/js/components/AdminStreamingRoomTypeEditButton.vueresources/js/components/RoomTabStreamingConfigButton.vueresources/js/components/SettingsFileSelector.vueresources/js/components/SettingsImageSelector.vueresources/js/views/AdminSettings.vueresources/js/views/AdminStreamingSettings.vuetests/Backend/Feature/api/v1/Room/RoomStreamingTest.phptests/Backend/Feature/api/v1/StreamingTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
- resources/js/components/RoomTabStreamingConfigButton.vue
- resources/js/components/SettingsFileSelector.vue
- app/Http/Requests/UpdateStreamingSettingsRequest.php
- resources/js/components/SettingsImageSelector.vue
- app/Http/Requests/UpdateSettingsRequest.php
There was a problem hiding this comment.
Pull request overview
This PR refactors file upload validation and error rendering across the Laravel API and Vue admin/room UIs, and adds consistent upload hints (allowed formats + max size) using shared helpers and i18n keys.
Changes:
- Standardize backend upload validation using fluent
Filerules and a newApp\Rules\Imagerule. - Update frontend upload components to show allowed formats/max size hints and to render errors via a unified
FormErrorAPI (arrays of messages). - Update fixtures and E2E/backend tests to match the new config shape and translated messages.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Frontend/fixtures/config.json | Updates BBB file MIME config fixture from comma string to array. |
| tests/Frontend/e2e/UserProfileBase.cy.js | Adjusts expected uploaded profile image filename/type. |
| tests/Frontend/e2e/RoomsViewFilesFileActions.cy.js | Updates expected i18n keys and size formatting for file upload hints/errors. |
| tests/Frontend/e2e/AdminUsersEditBase.cy.js | Adjusts expected uploaded profile image filename/type. |
| tests/Backend/Feature/api/v1/StreamingTest.php | Updates “wrong mime” fixture to align with new allowed image types. |
| tests/Backend/Feature/api/v1/SettingsTest.php | Updates config expectations for allowed_file_mimes from string to array. |
| tests/Backend/Feature/api/v1/Room/RoomStreamingTest.php | Updates “wrong mime” fixture to align with new allowed image types. |
| tests/Backend/Feature/api/v1/ApplicationTest.php | Updates config test expectations for BBB file mimes to arrays. |
| resources/js/views/Login.vue | Removes lazy tabs option to align with new login error handling flow. |
| resources/js/views/AdminStreamingSettings.vue | Updates streaming settings uploads (sizes/extensions) and helper text markup. |
| resources/js/views/AdminSettings.vue | Updates settings upload components to new error prop names and allowed extension arrays. |
| resources/js/views/AdminRoomTypesView.vue | Updates helper text markup for parameter descriptions. |
| resources/js/composables/useFormErrors.js | Refactors fieldError() to return arrays and updates its documentation. |
| resources/js/composables/useFileHelpers.js | Adds shared file size formatting helper. |
| resources/js/components/UserTabProfile.vue | Updates profile image upload filename to jpg. |
| resources/js/components/UserProfileImageSelector.vue | Tightens accepted extensions and wires PrimeVue upload error messages to new keys. |
| resources/js/components/SettingsImageSelector.vue | Refactors error rendering to arrays + adds allowed formats/max size hints. |
| resources/js/components/SettingsFileSelector.vue | Refactors error rendering to arrays + adds allowed formats/max size hints. |
| resources/js/components/RoomTabStreamingConfigButton.vue | Updates pause image validation props and helper text key. |
| resources/js/components/RoomTabSettingsToggleSwitch.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabSettingsTextArea.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabSettingsSelectButton.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabSettingsRoomTypeSelect.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabSettingsRadioGroup.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabSettingsAccessCodeInput.vue | Updates error prop typing to array. |
| resources/js/components/RoomTabFilesUploadButton.vue | Updates accept list generation, hint rendering, and “too large” wiring to new keys. |
| resources/js/components/LoginTabLocal.vue | Switches login error rendering to useFormErrors() and FormError arrays. |
| resources/js/components/LoginTabLdap.vue | Switches login error rendering to useFormErrors() and FormError arrays. |
| resources/js/components/FormError.vue | Changes errors prop expectation from object to array and updates logic accordingly. |
| resources/js/components/AdminStreamingRoomTypeEditButton.vue | Updates streaming room type pause image inputs to new props/keys and extensions. |
| lang/en/rooms.php | Removes room-specific upload hint strings and renames pause image helper key. |
| lang/en/app.php | Introduces shared app.file.* strings for upload hints and file validation messages. |
| config/bigbluebutton.php | Normalizes BBB_ALLOWED_FILE_MIMES into an array with a fallback list. |
| CHANGELOG.md | Adds entry for upload hints change and links PR reference. |
| app/Rules/Image.php | Adds reusable fluent File rule wrapper for image uploads. |
| app/Http/Requests/UserRequest.php | Switches profile image validation to fluent rules + new Image rule. |
| app/Http/Requests/UpdateStreamingSettingsRequest.php | Switches streaming pause image/css validation to fluent file rules. |
| app/Http/Requests/UpdateSettingsRequest.php | Switches multiple settings uploads to Image/fluent File rules. |
| app/Http/Requests/UpdateRoomTypeStreamingSettingsRequest.php | Switches pause image validation to Image rule + fluent dimensions. |
| app/Http/Requests/UpdateRoomStreamingConfigRequest.php | Switches pause image validation to Image rule + fluent dimensions. |
| app/Http/Requests/StoreRoomFileRequest.php | Switches room file validation to fluent File rules and new config shape. |
Comments suppressed due to low confidence (3)
resources/js/composables/useFormErrors.js:45
- Same JSDoc issue in
fieldError():{bool}should be{boolean}, and the phrase “errors that starting” should be “errors that start”.
* @param {string} field Name of field to get the error message for.
* @param {bool} [wildcard=false] Flag that indicates whether all errors that starting with the field name should be checked.
* @return {string[]} Empty array if there is no error for the passed field otherwise an array of strings with the messages for the passed field.
resources/js/components/RoomTabFilesUploadButton.vue:83
- Same issue as the
acceptbinding:.join()will throw ifbbb.file_mimesis not available yet. Provide a safe fallback to keep the dialog stable during initialization/error states.
$t("app.file.allowed_formats", {
formats: settingsStore.getSetting("bbb.file_mimes").join(", "),
})
resources/js/components/RoomTabFilesUploadButton.vue:88
bbb.max_filesizeis also retrieved viagetSetting()and can beundefined; multiplyingundefined * 1_000_000yieldsNaNand results in a broken size hint. Use a nullish fallback.
size: fileHelpers.fileSize(
settingsStore.getSetting("bbb.max_filesize") * 1_000_000,
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Type
Checklist
Changes
Other information
Summary by CodeRabbit